feat(input): add opt-in support for `input[range]` #15229

gkalpak · 2016-10-08T09:06:16Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature

What is the current behavior? (You can also link to an open issue here)
input[range] is not supported.

What is the new behavior (if this is a feature change)?
Support for input[range] can be opted into, by using the ng-range-input attribute.

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

Other information:
This is a POC. Docs need to be updated.

petebacondarwin · 2016-10-08T11:28:06Z

My initial comment is that we need to check that other things that were changed in these commits are not impacted, such as ngMax, ngMin, ngStep(?), etc.

gkalpak · 2016-10-08T12:05:48Z

With the exception of this line (which should not affect other inputs afaict), there has been no change in any input type other than range.
(There are a couple of minor refactorings, but no actual change.)

petebacondarwin · 2016-10-08T14:01:58Z

src/ng/directive/input.js

+function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
+  badInputChecker(scope, element, attr, ctrl);
+  numberFormatterParser(ctrl);
+  baseInputType(scope, element, attr, ctrl, $sniffer, $browser);


It probably doesn't make any difference but we should probably switch this line with the line above to ensure the order of the code is the same as previously.

petebacondarwin · 2016-10-08T14:05:38Z

src/ng/directive/ngModel.js

@@ -883,7 +883,8 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
        ctrl.$viewValue = ctrl.$$lastCommittedViewValue = viewValue;
        ctrl.$render();

-        ctrl.$$runValidators(modelValue, viewValue, noop);
+        // It is possible that model and view value have been updated during render
+        ctrl.$$runValidators(ctrl.$modelValue, ctrl.$viewValue, noop);


So this is not a breaking change because if $render didn't change the value then it will be the same as before, right? I think it is only the range input that is likely to do this...

Among the built-in $render function, yes. But the truth is it could be a breaking change for user defined $render functions (theoretically).

Although even in that case, it could be considered a fix 😁

petebacondarwin · 2016-10-08T14:07:01Z

test/ng/directive/inputSpec.js

        ['scheme://例子.测试', true],
-        ['scheme://उदाहरण.�रीक्षा', true],
+        ['scheme://उदाहरण.???ीक्षा', true],


Hmm, I wonder if this change is right? It looks like previously the strings were UTF-16?

Ooops! No idea how that sneaked in.

petebacondarwin · 2016-10-08T14:08:06Z

test/ng/directive/validatorsSpec.js

@@ -351,7 +351,7 @@ describe('validators', function() {


    it('should accept values of any length when maxlength is non-numeric', function() {
-      var inputElm = helper.compileInput('<input type="text" ng-model="value" ng-maxlength="{{maxlength}}" />');
+      var inputElm = helper.compileInput('<input type="text" ng-model="value" ng-maxlength="maxlength" />');


I guess that this was a bad test in the first place is not really part of this PR? Perhaps, if so, then when you are squashing all this, we move this change into its own commit?

petebacondarwin · 2016-10-08T14:09:24Z

OK, I think this is about right.
Let's check with @Narretz and then if @tbosch thinks it is useful we can fix up the docs and merge into 1.5

Narretz · 2016-10-09T18:16:26Z

I think it's pretty unorthodox of us to provide an opt-in for a regular input in core, especially since it becomes the default in 1.6. We are also forcing Googlers to update to 1.6 sooner if we leave it out of 1.5 ;) But the implementation is clean, so we might as well. Although I would like to see the refactor commits that affect other inputs in a separate commit.

This commit re-applies the related (previously reverted) commits. A follow-up commit will make the support opt-in in order to avoid a breaking change. Included commits: - 296da4b - `feat(input): add support for binding to input[type=range]` (previously reverted with 6a167e8) - b78539b - `fix(input[range]): correctly handle min/max; remove ngMin/ngMax support` (previously reverted with aa60491) - 90c08b8 - `feat(input[range]): support step` (previously reverted with 5b633d8)

gkalpak · 2016-10-10T15:48:03Z

I have tidied this up and fixed docs. (I also changed ng-range-input to ng-input-range as it seemed more consistent).

Closes #15229

petebacondarwin · 2016-10-10T17:30:49Z

Landed in 1.5.x as

Closes angular#15229

Closes #15229

googlebot added the cla: yes label Oct 8, 2016

gkalpak added type: feature component: forms frequency: low needs: review needs: public api severity: inconvenient branch: 1.5.x labels Oct 8, 2016

gkalpak added this to the 1.5.9 milestone Oct 8, 2016

gkalpak changed the title ~~Opt in support for input range~~ feat(input): add opt-in support for input[range] Oct 8, 2016

petebacondarwin reviewed Oct 8, 2016

View reviewed changes

gkalpak force-pushed the opt-in-support-for-input-range branch from 5e09fa7 to 60ccaca Compare October 8, 2016 14:24

gkalpak added 3 commits October 10, 2016 17:38

test(validators): fix broken test

9900b9b

fix(input[range]): make support for input[range] opt-in

c94a921

gkalpak force-pushed the opt-in-support-for-input-range branch from 60ccaca to c94a921 Compare October 10, 2016 15:44

gkalpak added a commit that referenced this pull request Oct 10, 2016

fix(input[range]): make support for input[range] opt-in

8830ee8

Closes #15229

petebacondarwin closed this Oct 10, 2016

gkalpak deleted the opt-in-support-for-input-range branch October 10, 2016 19:16

petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016

fix(input[range]): make support for input[range] opt-in

93594f7

Closes angular#15229

petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016

fix(input[range]): make support for input[range] opt-in

8511447

Closes angular#15229

petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016

fix(input[range]): make support for input[range] opt-in

7b0a43e

Closes angular#15229

petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016

fix(input[range]): make support for input[range] opt-in

109596f

Closes angular#15229

petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016

fix(input[range]): make support for input[range] opt-in

01883d9

Closes angular#15229

petebacondarwin pushed a commit that referenced this pull request Nov 23, 2016

fix(input[range]): make support for input[range] opt-in

58a5949

Closes #15229

petebacondarwin pushed a commit that referenced this pull request Nov 24, 2016

fix(input[range]): make support for input[range] opt-in

07b8761

Closes #15229

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(input): add opt-in support for `input[range]` #15229

feat(input): add opt-in support for `input[range]` #15229

Uh oh!

gkalpak commented Oct 8, 2016 •

edited

Loading

Uh oh!

petebacondarwin commented Oct 8, 2016

Uh oh!

gkalpak commented Oct 8, 2016

Uh oh!

petebacondarwin Oct 8, 2016

Uh oh!

petebacondarwin Oct 8, 2016

Uh oh!

gkalpak Oct 8, 2016

Uh oh!

gkalpak Oct 8, 2016

Uh oh!

petebacondarwin Oct 8, 2016

Uh oh!

gkalpak Oct 8, 2016

Uh oh!

petebacondarwin Oct 8, 2016

Uh oh!

petebacondarwin commented Oct 8, 2016 •

edited

Loading

Uh oh!

Narretz commented Oct 9, 2016

Uh oh!

gkalpak commented Oct 10, 2016

Uh oh!

petebacondarwin commented Oct 10, 2016

Uh oh!

Uh oh!

feat(input): add opt-in support for input[range] #15229

feat(input): add opt-in support for input[range] #15229

Uh oh!

Conversation

gkalpak commented Oct 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petebacondarwin commented Oct 8, 2016

Uh oh!

gkalpak commented Oct 8, 2016

Uh oh!

petebacondarwin Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

petebacondarwin Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

gkalpak Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

gkalpak Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

petebacondarwin Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

gkalpak Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

petebacondarwin Oct 8, 2016

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Oct 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Narretz commented Oct 9, 2016

Uh oh!

gkalpak commented Oct 10, 2016

Uh oh!

petebacondarwin commented Oct 10, 2016

Uh oh!

Uh oh!

feat(input): add opt-in support for `input[range]` #15229

feat(input): add opt-in support for `input[range]` #15229

gkalpak commented Oct 8, 2016 •

edited

Loading

petebacondarwin commented Oct 8, 2016 •

edited

Loading